Skip to content

test(integration): read seiload's pod before the follower catch-up wait - #601

Open
bdchatham wants to merge 1 commit into
mainfrom
devin/1791342674-seiload-log-before-catchup
Open

test(integration): read seiload's pod before the follower catch-up wait#601
bdchatham wants to merge 1 commit into
mainfrom
devin/1791342674-seiload-log-before-catchup

Conversation

@bdchatham

@bdchatham bdchatham commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a regression from #600. In runSeiload, assertSeiloadRun (seiload pod status + log summary) now runs before assertChainLive instead of after it:

runJob(ctx, t, cs, job)
-assertChainLive(ctx, t, hc, ch)
-assertSeiloadRun(ctx, t, cs, job, s)
+assertSeiloadRun(ctx, t, cs, job, s)
+assertChainLive(ctx, t, hc, ch)

Why: #600 lets assertChainLive wait up to followerCatchUpTimeout (5m) for followers. In the first nightly with it (nightly-harness-manual-1791341439), the catch-up worked: saturation rpc-1 went from about 230 blocks behind to caught up within about 3m. But the log read that came after it failed:

read seiload log: an error on the server ("unknown") has prevented the request from succeeding (get pods seiload-nightly-dly9jna8opln-saturation-lvwfq) — cannot verify the run

Prometheus shows the seiload pod went Succeeded around 03:05. By 03:06 its node ip-10-60-17-209 carried karpenter.sh/disrupted and node.kubernetes.io/not-ready, meaning Karpenter consolidated the emptied node and the kubelet serving the log was gone. Before #600 the log was read seconds after the Job finished, ahead of consolidation. A retry wouldn't help once the node is gone, so this change moves the read back to right after runJob.

The assertions are independent t.Errorf checks, so swapping them changes only when each one runs, not what fails.

Validation: go vet -tags integration ./test/integration/, gofmt, and diff-scoped golangci-lint are clean. Not verified on a nightly run yet. After merge, the platform integration-harness pin needs bumping again.

Link to Devin session: https://app.devin.ai/sessions/046d3f7315594005af4d7040d641c18f
Open in Devin Desktop: https://app.devin.ai/desktop/session/046d3f7315594005af4d7040d641c18f?variant=devin
Requested by: @bdchatham

@bdchatham @devin-ai-integration
test(integration): read seiload's pod before the follower catch-up wait
9cdc76b 
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

devin-ai-integration Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access that start with 'Devin'.

  • Disable automatic comment, CI, and merge conflict monitoring

@cursor

cursor Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only reordering in the integration harness; no production or runtime behavior changes.

Overview
Fixes a flaky integration failure where seiload pod log reads ran after assertChainLive's up-to-5m follower catch-up wait. Karpenter can consolidate the finished Job's node in that window, so get pods / log fetch fails even when the load run succeeded.

In runSeiload, assertSeiloadRun (pod termination + log summary checks) now runs immediately after runJob, then assertChainLive. Assertions stay independent; only ordering changes so logs are captured before long catch-up.

Reviewed by Cursor Bugbot for commit 9cdc76b. Bugbot is set up for automated code reviews on this repo. Configure here.

seidroid[bot]
seidroid Bot approved these changes Oct 7, 2026
View reviewed changes

@seidroid seidroid Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moves assertSeiloadRun ahead of assertChainLive in runSeiload, so seiload's pod status and log are read right after the Job completes instead of after the follower catch-up wait (up to 5m), during which Karpenter can reclaim the node. Both helpers report through t.Errorf and neither depends on the other's result, so only the timing changes; I found nothing blocking, and codex, the only prior reading, found nothing, which agrees with this review.

Non-blocking

  • This shrinks the race but does not remove it. waitJob polls every 10s, so the log read can still come some seconds after the pod reaches Succeeded, and a node Karpenter consolidates in that window still loses the log. To make the summary durable, have seiload write it to /dev/termination-log, where it survives in pod.Status.ContainerStatuses[].State.Terminated.Message, or ship it some other way that does not depend on the kubelet still serving logs.

seidroid review · decision approve · session 5cf6a32304764fe1911a2fb76ae31512 · turn resp_claude_b5dc958abffd2efa74373cfebc2dd408 · item 4854ae63a810573696ef1b90d3e4e97b

Findings: 0 blocking | 1 non-blocking | 0 posted inline

@devin-ai-integration

devin-ai-integration Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Agreed, this narrows the race rather than closing it. In the failing run the node was tainted about a minute after the pod went Succeeded, while waitJob's 10s poll puts the read within seconds, which is how it behaved before #600. A durable summary means changing seiload's output (e.g. writing to /dev/termination-log; FallbackToLogsOnError only applies to failed containers). That's a sei-load change, so I'm keeping this PR to the ordering fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seidroid seidroid[bot] seidroid[bot] approved these changes

Assignees

@bdchatham bdchatham

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@bdchatham